PWX-36477 : Updating Node PDB to allow k8s upgrades #1580
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## PWX-25414-24.2.0 #1580 +/- ##
====================================================
- Coverage 80.20% 80.13% -0.08%
====================================================
Files 61 61
Lines 18287 18464 +177
====================================================
+ Hits 14667 14796 +129
- Misses 2592 2624 +32
- Partials 1028 1044 +16 ☔ View full report in Codecov by Sentry. |
|
This PR is stale because it has been in review for 3 days with no activity. |
| @@ -13633,6 +13639,329 @@ func TestDeleteNodePodDisruptionBudget(t *testing.T) { | |||
|
|
|||
| } | |||
|
|
|||
| func TestUpdateNodePodDisruptionBudgetParallelEnabled(t *testing.T) { | |||
There was a problem hiding this comment.
These are good set of UTs, but I don't see every condition being covered. Eg. Decommissioned nodes, px node status not being okay, non-quorum member nodes cordoned, etc.
Let's go through the code that you have added line by line and make sure we have a test case of each condition. One case even for an ORed and ANDed sub-condition.
There was a problem hiding this comment.
added in UT cases for -
- decommissioned node
- px node status is down
- non quorum member also cordoned
- node without PDB cordoned
- node version empty
- When feature enabled, invalid minAvailable value, reconcile 2 times
- when feature disabled, invalid minAvailable, reconcile 2 times
|
@piyush-nimbalkar I also wanted to check if I can use the function - |
After looking at them, it doesn't look like they are exactly the same.
Does this make sense? |
* [Cherry-pick into feature]: Cherry-pick of all tickets related to Smart and parallel upgrades merged into master (#1578) * adding k8snode name label to px pods * correcting go fmt error * making requested changes * Creating node PDB * go fmt error * Checking node version instead of cluster * making requested changes Conflicts: drivers/storage/portworx/component/disruption_budget.go * aggregating errors * adding logic to delete node PDB * correcting go fmt errors * made requested changes * correcting after merge conflict * [cherry-pick] PWX-36509 : StorageCluster schema changes to support parallel portworx upgrades (#1576) * StorageCluster schema changes to support parallel portworx upgrades Signed-off-by: hitesh-wani-px <[email protected]> * change DisruptionSpec to Disruption and its description Signed-off-by: hitesh-wani-px <[email protected]> --------- Signed-off-by: hitesh-wani-px <[email protected]> * PWX-36477 : Updating Node PDB to allow k8s upgrades (#1580) * vendoring openstorage with nooverlappingnodes api * adding logic to update node PDB * Adding UTs and correcting logic * go fmt error * adding logic to delete cluster pdb after 3.1.2 * addressing comments requested * skip checking node version if version is empty * passing nodeIds instead of name to API * making requested changes and adding UTs * changing log to info * PWX-36510: No disruption of volumes for portworx upgrades (#1612) * non disruptive portworx upgrades * adding new method to driver interface * correcting UT * goimport fmt * gofmt file * update PX version in test * correcting new testcases --------- Signed-off-by: hitesh-wani-px <[email protected]> Co-authored-by: Swarupa Vijaykumar <[email protected]> Co-authored-by: hitesh-wani-px <[email protected]> Co-authored-by: svijaykumar-px <[email protected]>
What this PR does / why we need it: This PR takes care of updating the node PDBs for nodes that can be upgraded. First a list of cordoned nodes and nodes which have been selected for upgrade (PDB 0) are given as input to px api - FilterNonOverlappingNodes which gives an output of nodes that can be upgraded such that volume quorum is not lost. Then we check if user has provided any minAvailable value else we assume we have to maintain px quorum. If the provided value is invalid, we fall back to the quorum value. Then for every node in the response we change its PDB minAvailable value to 0 such that px quorum is maintained based on nodes that are already down and can go down.
In the case where user has disabled non disruptive upgrades, there is no call made to the api instead nodes are randomly selected for the upgrade based on minAvailable annotation provided by user. If this value is invalid, we only upgrade 1 node at a time.
Which issue(s) this PR fixes (optional)
Closes # PWX-36477, PWX-36478, PWX-36481, PWX-36504, PWX-36515
Special notes for your reviewer: UTs have been added for the cased but manual end to end testing is still pending